Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update fihooks.class.php #88

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Bournwog
Copy link

when I add 10-20 snippet calls in emailTpl, only 3 or 5 first calls will be processed

with this patch you can use correctly more than 5 snippets calls in emailTpl

parse more than 5 snippets call in emailTpl
@jpdevries
Copy link
Collaborator

Thanks for this @Bournwog. Are there any specific tests that come to mind to make sure this doesn't introduce any new issues?
I've got this merged into a testing branch over at the new repository location modxcms/FormIt
Sterc@42f4808

@bertoost
Copy link
Contributor

I know the parser a bit.. It seems to be a right thing :-) saw that stuff before 👍

@jpdevries
Copy link
Collaborator

@Bournwog why not use $this->modx->parser instead of calling $this->modx->getParser() twice?

@bertoost
Copy link
Contributor

bertoost commented Feb 5, 2014

What if the parser isn't loaded (theoretical it can be possible)?
I also agree, the getParser() should check that and only init a new instance when not already is loaded..
changed like:

public function getParser() {
    $parserClass = $this->getOption('parser_class', null, 'modParser');
    if(isset($this->parser) && is_object($this->parser) && $this->parser instanceof $parserClass) {
        return $this->parser;
    }
    return $this->getService('parser', $parserClass, $this->getOption('parser_class_path', null, ''));
}

@jpdevries
Copy link
Collaborator

@bertoost i'm talking about this in the PR

// parse all cacheable tags first
$this->modx->getParser()->processElementTags('', $str, true, false, '[[', ']]', array(), 10);
// parse all non-cacheable and remove unprocessed tags
$this->modx->getParser()->processElementTags('', $str, true, true, '[[', ']]', array(), 10);   

is there a problem with $this->modx->parser->processElementTags as it was before?

@bertoost
Copy link
Contributor

bertoost commented Feb 5, 2014

I think I copied this from a place somewhere else :-)
Don't know exactly.. it's been a while

@bertoost
Copy link
Contributor

bertoost commented Feb 5, 2014

Also; when I look logical;

$this->modx->getParser();
$this->modx->parser->processElementTags('', $str, true, false, '[[', ']]', array(), 10);
$this->modx->parser->processElementTags('', $str, true, false, '[[', ']]', array(), 10);

Should be fine too..

@jpdevries
Copy link
Collaborator

i guess i don't understand the need to call $this->modx->getParser(); since it wasn't doing so before. is it to ensure the parser is loaded?

@bertoost
Copy link
Contributor

bertoost commented Feb 5, 2014

Yes indeed. As said.. Theorethical it's possible that it isn't loaded. Always good to call the load method. That said; I also think the getParser() method should be improved, to avoid double loadings

@opengeek
Copy link
Collaborator

opengeek commented Feb 5, 2014

+1 on getParser() only getting an instance if not already loaded or a different parser class is specified

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants